-
Notifications
You must be signed in to change notification settings - Fork 444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkp/pkp-lib#2612 Submission filtering #2704
Conversation
b21da8f
to
2e37b80
Compare
…mplete submissions
2e37b80
to
dc1598f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for eventual illogical questions or if they do not make any sense... They are mostly just for me... This can be merged however...
@@ -1,12 +1,12 @@ | |||
<?php | |||
/** | |||
* @file controllers/list/submissions/SubmissionsListHandler.inc.php | |||
* @file controllers/list/submissions/PKPSubmissionsListHandler.inc.php | |||
* | |||
* Copyright (c) 2014-2017 Simon Fraser University | |||
* Copyright (c) 2000-2016 John Willinsky |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be 2017?
* | ||
* Copyright (c) 2014-2017 Simon Fraser University | ||
* Copyright (c) 2000-2016 John Willinsky | ||
* Distributed under the GNU GPL v2. For full terms see the file docs/COPYING. | ||
* | ||
* @class SubmissionsListHandler | ||
* @class PKPSubmissionsListHandler | ||
* @ingroup classes_controllers_list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is ingroup classes?
if (this.isFilterActive(type, val)) { | ||
this.clearFilter(type, val); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening here: when a filter that is active is added, it will be removed? :-\
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a toggle feature. Click once to activate it. If you click on the filter again, you deactivate it.
// Deactivate the isIncomplete filter when any other filter is selected | ||
} else if (this.isFilterActive('isIncomplete', true)) { | ||
this.clearFilter('isIncomplete', true); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the isIncomplete filter is always used alone -- it cannot be combined with others? -- Is this because the intersection with other filters does not exist, because it is always "AND" relationship for the results?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, isIncomplete
filter doesn't play well with the overdue or stage filters, because isIncomplete
is always in the Submission stage. But end-users don't really think of incomplete submissions as "in" the submission stage. They see it as a pre-stage.
* Can the current user filter the list? | ||
*/ | ||
currentUserCanFilter: function() { | ||
return pkp.userHasRole(['manager', 'subeditor', 'assistant']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: where is this function i.e. roles defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js/load.js
in OJS/OMP. Eventually we'll probably build the pkp
global from an export and can share code between OJS/OMP. But I'm keeping it simple for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and we'll probably use constants for this eventually. See #2708.
No description provided.